Thanks @joegrasse. If you add the check in setfperm() before overriding the permission we're good to go.

Actually I believe we don't want the check for -. In my scenario in #921, the original file would have returned -rw-rw-rw-. After editing the file, with my umask set to 0022, it would become -rw-r--r--. With my change above it stays -rw-rw-rw- after editing. So a - is actually a valid scenario.

The problem is that it should not set the permissions when getfperm returns a -. It's an edge case we don't want. So you should add an if clause just before setfperm to avoid this.

I sorry, I guess I am not following you. Are you thinking there is the case where it would return only -? Even if the file is chmoded as 000, it would still return ---------. What the doc is saying is, if the file doesn't has a specific permission it returns -. So if the file doesn't have the group writable permission it would give a - in that permission slot instead of w.

Passing "-" back into setfperm() turns the relevant permission off, which is the expected behavior if the original file also did not have that permission. Calling something like the following would essentially be a NOOP:

setfperm(file, getfperm(file))

However, in the reported bug, there are two separate files: the original one with the intended permissions and a temp file that has different (wrong) permissions due to the user's umask. When the temp file is moved over the original file, the temp file's permissions overwrite the original, intended permissions. This could lead to problems such as a world-writable file suddenly becoming read-only to everyone but the current user (or worse, it could happen the other way around).

This PR reapplies the permissions from the original file to the temp file exactly as they were. When the temp file is moved into place, it will have the same permissions as it did before calling fmt(), with no new permissions added and no previous permissions lost. Other than verifying that getfperm() does not return an empty string, no further checks or modifications of the perm string are necessary.

Do the last couple of comments clear your questions up for you, or do you still have concerns?

Thanks @joegrasse